Don't drop in-flight decode jobs when seeking in cram_next_slice() #1973
Conversation
cram_next_slice() with a region set tries to skip over containers with contents that don't overlap the region of interest. For files with a mixture of read lengths, it's possible that some decode jobs have been queued before a later container is skipped. This makes calling cram_seek() hazardous as it drops any in-flight jobs before updating the file position. Instead, call hseek() directly so already-queued decode jobs are retained.
The test creates a CRAM file with records in lots of containers, many of which should be skipped when making a region query that expects to return the first and last two records in the file.
|
My initial worry with this was why do we have a So it looks like there wasn't a specific concern over using system seeks. I do see that the original seek had more smarts and could do read-and-discard when operating on a pipe, which is something we lost when we switched to wrapping up hseek (deliberately: see 1f338eb). There have been other changes in the interim too, such as clearing the ooc flag (out of containers), although I note you do that too which is good. I'll investigate the other changes as I'm wondering if this is problematic elsewhere. |
|
I tested samtools with it, and also tested the issues reported in #655 which gave rise to the initial addition of the Looks good. Thanks |
2 participants
cram_next_slice()with a region set tries to skip over containers with contents that don't overlap the region of interest. For files with a mixture of read lengths, it's possible that some decode jobs have been queued before a later container is skipped. This makes callingcram_seek()hazardous as it drops any in-flight jobs before updating the file position. Instead, callhseek()directly so already-queued decode jobs are retained.Fixes samtools/samtools#2285 (CRAM region queries miss reads when using additional threads)